From 7a942be8c5c39487083af13be47e2144d66b44e0 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 12 Nov 2014 21:35:33 -0800 Subject: [PATCH] Stop shuffling output files around so often This commit is an architectural change inside of Cargo itself in the way that it handles the output format of builds. Previously when a build start, all existing directories and files would be renamed to `old-foo` folders. The build would then `rename` all files back into the right location as they were seen as fresh and needed for the build. The benefit of a system such as this is a rock-solid guarantee that the build tree contains exactly what it would if we were to start the build from a totally clean directory each time. There are some downsides, however: * In #800, it was discovered that this method has an unfortunate interaction with Docker. Docker apparently will mount many filesystems which `rename` will not work across. * I have seen countless flaky failures on windows due to an attempt to remove a file that was still in use somehow. I've never been able to truly track down why these failures are happening, however. The new system for managing output files is to build up a list of all known files at the start of a build, whitelist any necessary files when the build is being prepared, and then wipe out all unknown files right before the build begins. This is not quite as close to the guarantee as the benefits reaped before because on the second build all build files will still be in their final output locations, they may just get updated as part of the build as well. This seems like an acceptable compromise, however. Closes #800 --- src/cargo/ops/cargo_rustc/custom_build.rs | 25 ++-- src/cargo/ops/cargo_rustc/fingerprint.rs | 98 +++++++-------- src/cargo/ops/cargo_rustc/layout.rs | 147 ++++++++-------------- src/cargo/ops/cargo_rustc/mod.rs | 37 ++++-- 4 files changed, 128 insertions(+), 179 deletions(-) diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index a6e0261d7..2264044ec 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -39,11 +39,9 @@ pub struct BuildState { pub fn prepare(pkg: &Package, target: &Target, req: PlatformRequirement, cx: &mut Context) -> CargoResult<(Work, Work, Freshness)> { let kind = match req { PlatformPlugin => KindHost, _ => KindTarget, }; - let (script_output, build_output, old_build_output) = { - let target = cx.layout(pkg, kind); + let (script_output, build_output) = { (cx.layout(pkg, KindHost).build(pkg), - target.build_out(pkg), - target.proxy().old_build(pkg).join("out")) + cx.layout(pkg, KindTarget).build_out(pkg)) }; // Building the command to execute @@ -99,7 +97,6 @@ pub fn prepare(pkg: &Package, target: &Target, req: PlatformRequirement, let build_state = cx.build_state.clone(); let id = pkg.get_package_id().clone(); let all = (id.clone(), pkg_name.clone(), build_state.clone(), - old_build_output.clone(), build_output.clone()); try!(fs::mkdir_recursive(&cx.layout(pkg, KindTarget).build(pkg), USER_RWX)); @@ -115,14 +112,12 @@ pub fn prepare(pkg: &Package, target: &Target, req: PlatformRequirement, // // If we have an old build directory, then just move it into place, // otherwise create it! - try!(if old_build_output.exists() { - fs::rename(&old_build_output, &build_output) - } else { - fs::mkdir(&build_output, USER_RWX) - }.chain_error(|| { - internal("failed to create script output directory for \ - build command") - })); + if !build_output.exists() { + try!(fs::mkdir(&build_output, USER_RWX).chain_error(|| { + internal("failed to create script output directory for \ + build command") + })); + } // For all our native lib dependencies, pick up their metadata to pass // along to this custom build command. @@ -183,10 +178,8 @@ pub fn prepare(pkg: &Package, target: &Target, req: PlatformRequirement, try!(fingerprint::prepare_build_cmd(cx, pkg, Some(target))); let dirty = proc(tx: Sender) { try!(work(tx.clone())); dirty(tx) }; let fresh = proc(tx) { - let (id, pkg_name, build_state, old_build_output, build_output) = all; + let (id, pkg_name, build_state, build_output) = all; let new_loc = build_output.dir_path().join("output"); - try!(fs::rename(&old_build_output.dir_path().join("output"), &new_loc)); - try!(fs::rename(&old_build_output, &build_output)); let mut f = try!(File::open(&new_loc).map_err(|e| { human(format!("failed to read cached build command output: {}", e)) })); diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index e69734ed8..5949bfc58 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -1,7 +1,8 @@ use std::collections::hash_map::{Occupied, Vacant}; use std::hash::{Hash, Hasher}; use std::hash::sip::SipHasher; -use std::io::{fs, File, USER_RWX, BufferedReader}; +use std::io::{mod, fs, File, BufferedReader}; +use std::io::fs::PathExtensions; use core::{Package, Target}; use util; @@ -43,10 +44,9 @@ pub fn prepare_target(cx: &mut Context, pkg: &Package, target: &Target, kind: Kind) -> CargoResult { let _p = profile::start(format!("fingerprint: {} / {}", pkg.get_package_id(), target)); - let (old, new) = dirs(cx, pkg, kind); - let filename = filename(target); - let old_loc = old.join(filename.as_slice()); - let new_loc = new.join(filename.as_slice()); + let new = dir(cx, pkg, kind); + let loc = new.join(filename(target)); + cx.layout(pkg, kind).proxy().whitelist(&loc); // We want to use the package fingerprint if we're either a doc target or a // path source. If we're a git/registry source, then the mtime of files may @@ -58,13 +58,13 @@ pub fn prepare_target(cx: &mut Context, pkg: &Package, target: &Target, doc || !path }; - info!("fingerprint at: {}", new_loc.display()); + info!("fingerprint at: {}", loc.display()); // First bit of the freshness calculation, whether the dep-info file // indicates that the target is fresh. - let (old_dep_info, new_dep_info) = dep_info_loc(cx, pkg, target, kind); + let dep_info = dep_info_loc(cx, pkg, target, kind); let are_files_fresh = use_pkg || - try!(calculate_target_fresh(pkg, &old_dep_info)); + try!(calculate_target_fresh(pkg, &dep_info)); // Second bit of the freshness calculation, whether rustc itself, the // target are fresh, and the enabled set of features are all fresh. @@ -80,43 +80,38 @@ pub fn prepare_target(cx: &mut Context, pkg: &Package, target: &Target, } else { mk_fingerprint(cx, &(target, features)) }; - let is_rustc_fresh = try!(is_fresh(&old_loc, rustc_fingerprint.as_slice())); + let is_rustc_fresh = try!(is_fresh(&loc, rustc_fingerprint.as_slice())); - let (old_root, root) = { + let root = { let layout = cx.layout(pkg, kind); if target.get_profile().is_custom_build() { - (layout.old_build(pkg), layout.build(pkg)) + layout.build(pkg) } else if target.is_example() { - (layout.old_examples().clone(), layout.examples().clone()) + layout.examples().clone() } else { - (layout.old_root().clone(), layout.root().clone()) + layout.root().clone() } }; - let mut pairs = vec![(old_loc, new_loc.clone())]; if !target.get_profile().is_doc() { - pairs.push((old_dep_info, new_dep_info)); - for filename in try!(cx.target_filenames(target)).iter() { - let filename = filename.as_slice(); let dst = root.join(filename); - pairs.push((old_root.join(filename), root.join(filename))); + cx.layout(pkg, kind).proxy().whitelist(&dst); if target.get_profile().is_test() { - cx.compilation.tests.push((target.get_name().into_string(), dst.clone())); + cx.compilation.tests.push((target.get_name().into_string(), dst)); } else if target.is_bin() { - cx.compilation.binaries.push(dst.clone()); + cx.compilation.binaries.push(dst); } else if target.is_lib() { let pkgid = pkg.get_package_id().clone(); match cx.compilation.libraries.entry(pkgid) { Occupied(entry) => entry.into_mut(), Vacant(entry) => entry.set(Vec::new()), - }.push(root.join(filename)); + }.push(dst); } } } - Ok(prepare(is_rustc_fresh && are_files_fresh, new_loc, rustc_fingerprint, - pairs)) + Ok(prepare(is_rustc_fresh && are_files_fresh, loc, rustc_fingerprint)) } /// Prepare the necessary work for the fingerprint of a build command. @@ -147,76 +142,73 @@ pub fn prepare_build_cmd(cx: &mut Context, pkg: &Package, if pkg.get_manifest().get_build().len() == 0 && target.is_none() { return Ok((Fresh, proc(_) Ok(()), proc(_) Ok(()))) } - let (old, new) = dirs(cx, pkg, kind); - let old_loc = old.join("build"); - let new_loc = new.join("build"); + let new = dir(cx, pkg, kind); + let loc = new.join("build"); + cx.layout(pkg, kind).proxy().whitelist(&loc); - info!("fingerprint at: {}", new_loc.display()); + info!("fingerprint at: {}", loc.display()); let new_fingerprint = try!(calculate_build_cmd_fingerprint(cx, pkg)); let new_fingerprint = mk_fingerprint(cx, &new_fingerprint); - let is_fresh = try!(is_fresh(&old_loc, new_fingerprint.as_slice())); - let mut pairs = vec![(old_loc, new_loc.clone())]; + let is_fresh = try!(is_fresh(&loc, new_fingerprint.as_slice())); // The new custom build command infrastructure handles its own output // directory as part of freshness. if target.is_none() { let native_dir = cx.layout(pkg, kind).native(pkg); - pairs.push((cx.layout(pkg, kind).old_native(pkg), native_dir.clone())); cx.compilation.native_dirs.insert(pkg.get_package_id().clone(), native_dir); } - Ok(prepare(is_fresh, new_loc, new_fingerprint, pairs)) + Ok(prepare(is_fresh, loc, new_fingerprint)) } /// Prepare work for when a package starts to build pub fn prepare_init(cx: &mut Context, pkg: &Package, kind: Kind) -> (Work, Work) { - let (_, new1) = dirs(cx, pkg, kind); + let new1 = dir(cx, pkg, kind); let new2 = new1.clone(); - let work1 = proc(_) { try!(fs::mkdir(&new1, USER_RWX)); Ok(()) }; - let work2 = proc(_) { try!(fs::mkdir(&new2, USER_RWX)); Ok(()) }; + let work1 = proc(_) { + if !new1.exists() { + try!(fs::mkdir(&new1, io::USER_DIR)); + } + Ok(()) + }; + let work2 = proc(_) { + if !new2.exists() { + try!(fs::mkdir(&new2, io::USER_DIR)); + } + Ok(()) + }; (work1, work2) } /// Given the data to build and write a fingerprint, generate some Work /// instances to actually perform the necessary work. -fn prepare(is_fresh: bool, loc: Path, fingerprint: String, - to_copy: Vec<(Path, Path)>) -> Preparation { +fn prepare(is_fresh: bool, loc: Path, fingerprint: String) -> Preparation { let write_fingerprint = proc(desc_tx) { drop(desc_tx); try!(File::create(&loc).write_str(fingerprint.as_slice())); Ok(()) }; - let move_old = proc(desc_tx) { - drop(desc_tx); - for &(ref src, ref dst) in to_copy.iter() { - try!(fs::rename(src, dst)); - } - Ok(()) - }; - - (if is_fresh {Fresh} else {Dirty}, write_fingerprint, move_old) + (if is_fresh {Fresh} else {Dirty}, write_fingerprint, proc(_) Ok(())) } /// Return the (old, new) location for fingerprints for a package -pub fn dirs(cx: &Context, pkg: &Package, kind: Kind) -> (Path, Path) { - let layout = cx.layout(pkg, kind); - let layout = layout.proxy(); - (layout.old_fingerprint(pkg), layout.fingerprint(pkg)) +pub fn dir(cx: &Context, pkg: &Package, kind: Kind) -> Path { + cx.layout(pkg, kind).proxy().fingerprint(pkg) } /// Returns the (old, new) location for the dep info file of a target. pub fn dep_info_loc(cx: &Context, pkg: &Package, target: &Target, - kind: Kind) -> (Path, Path) { - let (old, new) = dirs(cx, pkg, kind); - let filename = format!("dep-{}", filename(target)); - (old.join(filename.as_slice()), new.join(filename)) + kind: Kind) -> Path { + let ret = dir(cx, pkg, kind).join(format!("dep-{}", filename(target))); + cx.layout(pkg, kind).proxy().whitelist(&ret); + return ret; } fn is_fresh(loc: &Path, new_fingerprint: &str) -> CargoResult { diff --git a/src/cargo/ops/cargo_rustc/layout.rs b/src/cargo/ops/cargo_rustc/layout.rs index a1bbb695e..2b277a558 100644 --- a/src/cargo/ops/cargo_rustc/layout.rs +++ b/src/cargo/ops/cargo_rustc/layout.rs @@ -43,30 +43,13 @@ //! # Hidden directory that holds all of the fingerprint files for all //! # packages //! .fingerprint/ -//! -//! # This is a temporary directory as part of the build process. When a -//! # build starts, it initially moves the old `deps` directory to this -//! # location. This is done to ensure that there are no stale artifacts -//! # lying around in the build directory which may cause a build to -//! # succeed where it would fail elsewhere. -//! # -//! # If a package is determined to be fresh, its files are moved out of -//! # this directory and back into `deps`. -//! old-deps/ -//! -//! # Similar to old-deps, this is where all of the output under -//! # `target/` is moved at the start of a build. -//! old-root/ -//! -//! # Same as the two above old directories -//! old-native/ -//! old-build/ -//! old-fingerprint/ -//! old-examples/ //! ``` -use std::io::{mod, fs, IoResult}; +use std::cell::RefCell; +use std::collections::HashSet; use std::io::fs::PathExtensions; +use std::io::{mod, fs, IoResult}; +use std::mem; use core::Package; use util::hex::short_hash; @@ -78,13 +61,7 @@ pub struct Layout { build: Path, fingerprint: Path, examples: Path, - - old_deps: Path, - old_root: Path, - old_native: Path, - old_build: Path, - old_fingerprint: Path, - old_examples: Path, + to_delete: RefCell>, } pub struct LayoutProxy<'a> { @@ -113,13 +90,8 @@ impl Layout { build: root.join("build"), fingerprint: root.join(".fingerprint"), examples: root.join("examples"), - old_deps: root.join("old-deps"), - old_root: root.join("old-root"), - old_native: root.join("old-native"), - old_build: root.join("old-build"), - old_fingerprint: root.join("old-fingerprint"), - old_examples: root.join("old-examples"), root: root, + to_delete: RefCell::new(HashSet::new()), } } @@ -128,36 +100,35 @@ impl Layout { try!(fs::mkdir_recursive(&self.root, io::USER_RWX)); } - try!(old(&[ - (&self.old_deps, &self.deps), - (&self.old_native, &self.native), - (&self.old_fingerprint, &self.fingerprint), - (&self.old_examples, &self.examples), - (&self.old_build, &self.build), - ])); + try!(mkdir(self, &self.deps, false)); + try!(mkdir(self, &self.native, false)); + try!(mkdir(self, &self.fingerprint, true)); + try!(mkdir(self, &self.examples, false)); + try!(mkdir(self, &self.build, false)); - if self.old_root.exists() { - try!(fs::rmdir_recursive(&self.old_root)); - } - try!(fs::mkdir(&self.old_root, io::USER_RWX)); - - for file in try!(fs::readdir(&self.root)).iter() { + for file in try!(fs::readdir(&self.root)).into_iter() { if !file.is_file() { continue } - try!(fs::rename(file, &self.old_root.join(file.filename().unwrap()))); + self.to_delete.borrow_mut().insert(file); } return Ok(()); - fn old(dirs: &[(&Path, &Path)]) -> IoResult<()> { - for &(old, new) in dirs.iter() { - if old.exists() { - try!(fs::rmdir_recursive(old)); - } - if new.exists() { - try!(fs::rename(new, old)); + fn mkdir(layout: &Layout, dir: &Path, deep: bool) -> IoResult<()> { + if dir.exists() { + if deep { + for file in try!(fs::walk_dir(dir)) { + if !file.is_dir() { + layout.to_delete.borrow_mut().insert(file); + } + } + } else { + for file in try!(fs::readdir(dir)).into_iter() { + layout.to_delete.borrow_mut().insert(file); + } } - try!(fs::mkdir(new, io::USER_DIR)); + } else { + try!(fs::mkdir(dir, io::USER_DIR)); } Ok(()) } @@ -169,34 +140,42 @@ impl Layout { // TODO: deprecated, remove pub fn native(&self, package: &Package) -> Path { - self.native.join(self.pkg_dir(package)) + let ret = self.native.join(self.pkg_dir(package)); + self.whitelist(&ret); + ret } pub fn fingerprint(&self, package: &Package) -> Path { - self.fingerprint.join(self.pkg_dir(package)) + let ret = self.fingerprint.join(self.pkg_dir(package)); + self.whitelist(&ret); + ret } pub fn build(&self, package: &Package) -> Path { - self.build.join(self.pkg_dir(package)) + let ret = self.build.join(self.pkg_dir(package)); + self.whitelist(&ret); + ret } pub fn build_out(&self, package: &Package) -> Path { self.build(package).join("out") } - pub fn old_dest<'a>(&'a self) -> &'a Path { &self.old_root } - pub fn old_deps<'a>(&'a self) -> &'a Path { &self.old_deps } - pub fn old_examples<'a>(&'a self) -> &'a Path { &self.old_examples } - - // TODO: deprecated, remove - pub fn old_native(&self, package: &Package) -> Path { - self.old_native.join(self.pkg_dir(package)) - } - pub fn old_fingerprint(&self, package: &Package) -> Path { - self.old_fingerprint.join(self.pkg_dir(package)) + pub fn clean(&self) -> IoResult<()> { + let set = mem::replace(&mut *self.to_delete.borrow_mut(), + HashSet::new()); + for file in set.iter() { + info!("dirty: {}", file.display()); + if file.is_dir() { + try!(fs::rmdir_recursive(file)); + } else { + try!(fs::unlink(file)); + } + } + Ok(()) } - pub fn old_build(&self, package: &Package) -> Path { - self.old_build.join(self.pkg_dir(package)) + pub fn whitelist(&self, p: &Path) { + self.to_delete.borrow_mut().remove(p); } fn pkg_dir(&self, pkg: &Package) -> String { @@ -204,17 +183,6 @@ impl Layout { } } -impl Drop for Layout { - fn drop(&mut self) { - let _ = fs::rmdir_recursive(&self.old_deps); - let _ = fs::rmdir_recursive(&self.old_root); - let _ = fs::rmdir_recursive(&self.old_native); - let _ = fs::rmdir_recursive(&self.old_fingerprint); - let _ = fs::rmdir_recursive(&self.old_examples); - let _ = fs::rmdir_recursive(&self.old_build); - } -} - impl<'a> LayoutProxy<'a> { pub fn new(root: &'a Layout, primary: bool) -> LayoutProxy<'a> { LayoutProxy { @@ -237,20 +205,5 @@ impl<'a> LayoutProxy<'a> { pub fn build_out(&self, pkg: &Package) -> Path { self.root.build_out(pkg) } - pub fn old_root(&self) -> &'a Path { - if self.primary {self.root.old_dest()} else {self.root.old_deps()} - } - - pub fn old_examples(&self) -> &'a Path { self.root.old_examples() } - - // TODO: deprecated, remove - pub fn old_native(&self, pkg: &Package) -> Path { - self.root.old_native(pkg) - } - - pub fn old_build(&self, pkg: &Package) -> Path { - self.root.old_build(pkg) - } - pub fn proxy(&self) -> &'a Layout { self.root } } diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index fe7304ce8..bcea5a073 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -141,6 +141,10 @@ pub fn compile_targets<'a>(env: &str, targets: &[&'a Target], pkg: &'a Package, try!(compile(targets, pkg, true, &mut cx, &mut queue)); + // Clean out any old files sticking around in directories. + try!(cx.layout(pkg, KindHost).proxy().clean()); + try!(cx.layout(pkg, KindTarget).proxy().clean()); + // Now that we've figured out everything that we're going to do, do it! try!(queue.execute(cx.config)); @@ -316,7 +320,6 @@ fn compile_custom_old(pkg: &Package, cmd: &str, // may be building a C lib for a plugin let layout = cx.layout(pkg, KindTarget); let output = layout.native(pkg); - let old_output = layout.proxy().old_native(pkg); let mut p = try!(process(cmd.next().unwrap(), pkg, target, cx)) .env("OUT_DIR", Some(&output)) .env("DEPS_DIR", Some(&output)) @@ -353,14 +356,10 @@ fn compile_custom_old(pkg: &Package, cmd: &str, Ok(proc(desc_tx: Sender) { desc_tx.send_opt(p.to_string()).ok(); - if first { - try!(if old_output.exists() { - fs::rename(&old_output, &output) - } else { - fs::mkdir(&output, USER_RWX) - }.chain_error(|| { + if first && !output.exists() { + try!(fs::mkdir(&output, USER_RWX).chain_error(|| { internal("failed to create output directory for build command") - })); + })) } try!(p.exec_with_output().map(|_| ()).map_err(|mut e| { e.msg = format!("Failed to run custom build command for `{}`\n{}", @@ -377,13 +376,16 @@ fn rustc(package: &Package, target: &Target, let crate_types = target.rustc_crate_types(); let rustcs = try!(prepare_rustc(package, target, crate_types, cx, req)); - Ok(rustcs.into_iter().map(|(rustc, kind)| { + rustcs.into_iter().map(|(rustc, kind)| { let name = package.get_name().to_string(); let is_path_source = package.get_package_id().get_source_id().is_path(); let show_warnings = package.get_package_id() == cx.resolve.root() || is_path_source; let rustc = if show_warnings {rustc} else {rustc.arg("-Awarnings")}; + let filenames = try!(cx.target_filenames(target)); + let root = cx.layout(package, kind).root().clone(); + // Prepare the native lib state (extra -L and -l flags) let build_state = cx.build_state.clone(); let mut native_lib_deps = HashSet::new(); @@ -416,7 +418,7 @@ fn rustc(package: &Package, target: &Target, t.is_lib() }); - (proc(desc_tx: Sender) { + Ok((proc(desc_tx: Sender) { let mut rustc = rustc; // Only at runtime have we discovered what the extra -L and -l @@ -436,6 +438,15 @@ fn rustc(package: &Package, target: &Target, } } + // FIXME(rust-lang/rust#18913): we probably shouldn't have to do + // this manually + for filename in filenames.iter() { + let dst = root.join(filename); + if dst.exists() { + try!(fs::unlink(&dst)); + } + } + desc_tx.send_opt(rustc.to_string()).ok(); try!(rustc.exec().chain_error(|| { human(format!("Could not compile `{}`.", name)) @@ -443,8 +454,8 @@ fn rustc(package: &Package, target: &Target, Ok(()) - }, kind) - }).collect()) + }, kind)) + }).collect() } fn prepare_rustc(package: &Package, target: &Target, crate_types: Vec<&str>, @@ -622,7 +633,7 @@ fn build_plugin_args(mut cmd: ProcessBuilder, cx: &Context, pkg: &Package, cmd = cmd.arg("--out-dir"); cmd = cmd.arg(out_dir); - let (_, dep_info_loc) = fingerprint::dep_info_loc(cx, pkg, target, kind); + let dep_info_loc = fingerprint::dep_info_loc(cx, pkg, target, kind); cmd = cmd.arg("--dep-info").arg(dep_info_loc); if kind == KindTarget { -- 2.30.2